Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add keepMounted and itemProps to Vue API to enable sticky groups #576

Merged
merged 2 commits into from
Dec 14, 2024

Conversation

AmitJoki
Copy link
Contributor

@AmitJoki AmitJoki commented Dec 9, 2024

I wanted to use virtua for the usecase of sticky groups + virtualized scrolls in Vue but found that the APIs for that is not yet there.

I have added keepMounted from the React API and an additional itemProps to facilitate the Sticky Groups in Vue.

I have also added a story for the same.

If the PR is okay in its direction, I can add some documentation as well if needed.

Copy link
Owner

@inokawa inokawa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for the PR.

The docs will be autogenerated from JSDoc so you don't need to modify /docs dir.

Comment on lines 46 to 49
/**
* A function that provides properties/attributes for item element
*/
itemProps: Function as PropType<ItemProps>,
Copy link
Owner

@inokawa inokawa Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you try existed item prop instead of adding new prop? I don't think we need 2 props for 1 purpose.

Suggested change
/**
* A function that provides properties/attributes for item element
*/
itemProps: Function as PropType<ItemProps>,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refs work a bit differently in Vue https://vuejs.org/guide/essentials/template-refs and there's no equivalent of forwardRef

Also, on native elements, the ref correctly points to the DOM element but for a Vue component, the structure changes and the ref points to the component's instance and you have to use .$el to actually get the DOM element.

Also while JSX is "supported", SFCs are the first-class way of building Vue applications and there refs are extracted from the template.

The item prop only accepts the native element and not user component like it does in React API perhaps for the reasons laid out above.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the replay.

Also while JSX is "supported", SFCs are the first-class way of building Vue applications and there refs are extracted from the template.

Okay, we have a plan to migrate to SFC (#556). So we need to complete it at first, and change item prop to accept element and component. You mean it's possible with SFC, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@inokawa yes it is possible. Here's how one could do it using ref function: Playground

VirtualItem component accepts item prop which can be a string for native elements or the component itself. The setRef function will distinguish between the two and give you the correct DOM element be it a native element or a user component.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AmitJoki
Thank you. It's very helpful!

And I'm sorry for my late reply. Migrating to SFC may take a lot of time so I'll merge this PR anyway. And I may deprecate itemProps and merge it into item in near future.

@inokawa inokawa merged commit 89005a2 into inokawa:main Dec 14, 2024
4 of 5 checks passed
@inokawa
Copy link
Owner

inokawa commented Dec 14, 2024

Released in 0.39.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants